-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CASSANDRA-14572 Expose all table metrics in virtual tables #2958
base: trunk
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we not have this java file under src/java/
please.
like the test/anttasks/org/apache/cassandra/anttasks/
files, a separate directory structure would be nice (intuitive it's a separate compile and run classpath)
no good suggestions, maybe test/annotation-processor/src/
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the SystemViewAnnotationProcessor reduces the need for boilerplate code for some additional model classes, it does add complexity to the build scripts. This is a trade-off that we need to agree on.
For me, keeping all the generated classes sounds slightly better than increasing the complexity of the build, as the build.xml is already quite large. Do you think it's better to remove the SystemViewAnnotationProcessor
or is it better to keep it and move it to the path you've suggested?
The rationale is here, option #1 from here.
https://issues.apache.org/jira/browse/CASSANDRA-14572?focusedCommentId=17800171&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17800171
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm +0 towards not keeping the generated classes. And the build complexity can hopefully be all encapsulated in a separate .build/build-compile-annotations.xml
file.
If it is kept, then keep it elsewhere than the main src/java/
please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let's leave it as is for now. I moved it to test/annotation-processor
, but we don't have a good place for it, as it's neither the tools
nor the tests
are suitable.
src/resources/META-INF/services/javax.annotation.processing.Processor
Outdated
Show resolved
Hide resolved
src/java/org/apache/cassandra/db/virtual/BatchMetricsTable.java
Outdated
Show resolved
Hide resolved
bf5f1b6
to
0ee2f0a
Compare
0ee2f0a
to
a596b2d
Compare
Thanks for sending a pull request! Here are some tips if you're new here:
Commit messages should follow the following format:
The Cassandra Jira